Skip to content

Conversation

@pwendell
Copy link
Contributor

This patch marks some existing classes as private[spark] and adds two types of API annotations:

  • EXPERIMENTAL API = experimental user-facing module
  • DEVELOPER API - UNSTABLE = developer-facing API that might change

There is some discussion of the different mechanisms for doing this here:
https://issues.apache.org/jira/browse/SPARK-1081

I was pretty aggressive with marking things private. Keep in mind that if we want to open something up in the future we can, but we can never reduce visibility.

A few notes here:

  • In the past we've been inconsistent with the visiblity of the X-RDD classes. This patch marks them private whenever there is an existing function in RDD that can directly creat them (e.g. CoalescedRDD and rdd.coalesce()). One trade-off here is users can't subclass them.
  • Noted that compression and serialization formats don't have to be wire compatible across versions.
  • Compression codecs and serialization formats are semi-private as users typically don't instantiate them directly.
  • Metrics sources are made private - user only interacts with them through Spark's reflection

@AmplabJenkins
Copy link

Merged build triggered. Build is starting -or- tests failed to complete.

@AmplabJenkins
Copy link

Merged build started. Build is starting -or- tests failed to complete.

@AmplabJenkins
Copy link

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13589/

@mateiz
Copy link
Contributor

mateiz commented Apr 3, 2014

Hey Patrick, a few comments:

  • Should Logging become private[spark]? I think we looked at it before and there was some weird reason why we didn't do it, not sure what.
  • The badges unfortunately display weirdly in summary pages. For example look at this one for the root package. The first badge there is actually for Aggregator, but it displays on the bottom of its line, which makes it look like it might belong to the next element.
    screen shot 2014-04-02 at 5 37 23 pm
  • It might be good to give these different colors, e.g. something blue for Experimental API.

@mateiz
Copy link
Contributor

mateiz commented Apr 3, 2014

Some other classes that may need to be annotated:

  • InterruptibleIterator -- probably private[spark]
  • RDD.*Approx (e.g. countApprox), and similar methods in JavaRDD, DoubleRDD, PairRDD -- experimental
  • RDD.mapPartitionsWithContext -- developer
  • JobLogger -- developer (I also saw it's deprecated)
  • scheduler.SchedulingMode -- private[spark]
  • scheduler.SplitInfo -- either private[spark] or developer (if it appears in events)
  • storage.StoragePerfTester -- should probably just be moved to the Spark Tools project
  • util.Vector -- deprecate this, MLlib will have its own, better ones
  • util.BoundedPriorityQueue -- any reason this is not just private[spark]?
  • SimpleFutureAction -- developer
  • SparkContext.addSparkListener -- developer
  • SparkContext.runApproximateJob -- developer
  • SparkContext.runJob -- developer
  • SparkContext.submitJob -- experimental
  • SparkContext.warnSparkMem -- probably need to be private
  • broadcast.BroadcastFactory -- developer
  • RDD.compute, dependencies, iterator and such should probably also be developer

@mateiz
Copy link
Contributor

mateiz commented Apr 3, 2014

BTW to fix the floating badge problem, you might do the following: change code like this:

  <span class="badge badge-red" style="float: right;">DEVELOPER API - UNSTABLE</span>

   Represents a one-to-one dependency between partitions of the parent and child RDDs.

To this:

  <span class="badge badge-red" style="float: right;">DEVELOPER API - UNSTABLE
  </span>Represents a one-to-one dependency between partitions of the parent and child RDDs.

I believe Scaladoc includes only the first sentence it finds, so this might make it include both the text and the floating span. It might also not work though. But the reason those are displaying weirdly is partly that the first line there is ignored.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to relax this since I don't think the user cannot construct Product2 version of the CoGroupedRDD in PairRDDFunctions

@AmplabJenkins
Copy link

Build triggered.

@AmplabJenkins
Copy link

Build started.

@AmplabJenkins
Copy link

Build finished.

@AmplabJenkins
Copy link

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13806/

@AmplabJenkins
Copy link

Build triggered.

@AmplabJenkins
Copy link

Build started.

@AmplabJenkins
Copy link

Build finished.

@mengxr
Copy link
Contributor

mengxr commented Apr 9, 2014

That is the artifact name. Java uses annotation: http://docs.oracle.com/javase/7/docs/api/java/lang/annotation/Documented.html

@pwendell
Copy link
Contributor Author

pwendell commented Apr 9, 2014

Okay - I'll change this to annotation

@AmplabJenkins
Copy link

Build triggered.

@AmplabJenkins
Copy link

Build started.

@AmplabJenkins
Copy link

Build finished. All automated tests passed.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13931/

Conflicts:
	core/src/main/scala/org/apache/spark/scheduler/JobResult.scala
	core/src/main/scala/org/apache/spark/storage/StorageUtils.scala
	core/src/main/scala/org/apache/spark/util/TimeStampedHashMap.scala
	sql/core/src/main/scala/org/apache/spark/sql/SchemaRDD.scala
@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13939/

@pwendell
Copy link
Contributor Author

pwendell commented Apr 9, 2014

Just merged this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants